-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add support for email alert channel #4599
Conversation
WalkthroughThe recent updates introduce email alert channels across various plans, with functionalities for creating, editing, and testing email configurations. These changes enable direct email alerting, addressing the need for broader notification options beyond existing channels. The modifications span across backend and frontend, including adjustments to plan features, API endpoints for email channel management, and UI components for configuring email alerts. Changes
Assessment against linked issues
PoemAs the dawn breaks and the code compiles,
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
frontend/public/locales/en/channels.json
is excluded by:!**/*.json
Files selected for processing (14)
- ee/query-service/model/plans.go (4 hunks)
- frontend/src/api/channels/createEmail.ts (1 hunks)
- frontend/src/api/channels/editEmail.ts (1 hunks)
- frontend/src/api/channels/testEmail.ts (1 hunks)
- frontend/src/container/CreateAlertChannels/config.ts (1 hunks)
- frontend/src/container/CreateAlertChannels/defaults.ts (2 hunks)
- frontend/src/container/CreateAlertChannels/index.tsx (10 hunks)
- frontend/src/container/EditAlertChannels/index.tsx (8 hunks)
- frontend/src/container/FormAlertChannels/Settings/Email.tsx (1 hunks)
- frontend/src/container/FormAlertChannels/index.tsx (5 hunks)
- frontend/src/pages/ChannelsEdit/index.tsx (1 hunks)
- frontend/src/types/api/channels/createEmail.ts (1 hunks)
- frontend/src/types/api/channels/editEmail.ts (1 hunks)
- pkg/query-service/model/featureSet.go (2 hunks)
Additional comments: 24
frontend/src/types/api/channels/createEmail.ts (1)
- 1-8: The type definitions for creating an email channel look good. However, consider refining the type of the
data
field inPayloadProps
if the structure of the response payload is known and can be more specifically typed.frontend/src/types/api/channels/editEmail.ts (1)
- 1-10: The type definitions for editing an email channel are well-structured. As with the createEmail types, consider refining the type of the
data
field inPayloadProps
if the structure of the response payload is known and can be more specifically typed.frontend/src/api/channels/createEmail.ts (1)
- 7-34: The implementation of the
create
function for creating an email channel is correct and follows best practices for making API requests and error handling. Consider adding type annotations for the function parameters and return type to enhance type safety and code readability.frontend/src/api/channels/testEmail.ts (1)
- 7-34: The
testEmail
function implementation is correct, with appropriate API request structure and error handling. As with the create function, adding type annotations for the function parameters and return type would improve type safety and code readability.frontend/src/api/channels/editEmail.ts (1)
- 7-34: The
editEmail
function for editing an email channel is correctly implemented, with proper API request structure and error handling. Adding type annotations for the function parameters and return type is recommended to enhance type safety and code readability.ee/query-service/model/plans.go (4)
- 93-99: Adding the
AlertChannelEmail
feature withActive: true
andUsage: 0
to theBasicPlan
is a significant enhancement. This change aligns with the PR objectives to introduce email alert channel support. Ensure that the defaultUsageLimit
of-1
correctly represents unlimited usage for this feature, as this could impact how users interact with the email alert functionality.- 187-193: The inclusion of
AlertChannelEmail
in theProPlan
withActive: true
extends the email alert functionality to more users. Similar to theBasicPlan
, confirm that theUsageLimit: -1
is intended to signify unlimited usage. This consistency across plans is crucial for a seamless user experience.- 281-287: Integrating
AlertChannelEmail
into theEnterprisePlan
ensures that the highest tier of users also benefits from the new email alert channel. As with the other plans, theUsageLimit: -1
should be verified to ensure it aligns with the intended unlimited usage policy for enterprise users.- 303-314: The modifications to the
Onboarding
andChatSupport
features in theEnterprisePlan
, setting both toActive: true
, enhance the support and onboarding experience for enterprise users. It's important to ensure these changes are well-documented and communicated to users to maximize their impact.frontend/src/container/EditAlertChannels/index.tsx (5)
- 2-8: The addition of
editEmail
andtestEmail
imports is essential for supporting the new email alert channel functionality. Ensure that these API functions are correctly implemented and tested to handle email configurations effectively.- 17-17: Including
EmailChannel
in the list of channel types is a crucial step in integrating email alerts into the UI. This change allows users to select and configure email as an alert channel, enhancing the platform's flexibility.- 163-191: The implementation of
prepareEmailRequest
andonEmailEditHandler
functions, along with their integration into the main edit flow, is well-executed. It's important to validate the email configuration fields (name
,to
,html
,headers
) for correctness and security (e.g., preventing injection attacks). Additionally, consider adding client-side validation for the email address format to improve user experience and reduce server-side errors.- 334-341: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [337-347]
Integrating the email edit handler into the main save flow ensures that changes to email alert configurations are correctly processed and saved. This seamless integration is key to providing a consistent and user-friendly experience across different alert channel types.
- 375-384: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [378-417]
The addition of email channel testing functionality through
performChannelTest
and the integration ofprepareEmailRequest
for testing purposes is a valuable feature. It allows users to verify their email alert configurations before saving them. Ensure that error handling and user feedback mechanisms are in place for the testing process to help users troubleshoot potential configuration issues.frontend/src/container/CreateAlertChannels/index.tsx (10)
- 2-2: The addition of
createEmail
import aligns with the implementation of email alert channels. Ensure that thecreateEmail
API function is thoroughly tested to handle various email configurations and error scenarios.- 8-8: The addition of
testEmail
import is crucial for testing the email alert configuration before saving. It's important to verify that thetestEmail
function properly validates email parameters and handles errors gracefully.- 23-23: The
EmailChannel
type addition is necessary for typing the email channel configuration. Ensure that all required fields for an email alert channel are covered and correctly typed.- 32-32: The
EmailInitialConfig
provides default values for the email channel configuration. Review the default configuration to ensure it aligns with typical use cases and doesn't contain sensitive defaults.- 106-112: The logic to reset the configuration to email defaults when the channel type changes to email is correctly implemented. Ensure that the
EmailInitialConfig
covers all necessary fields for a basic email alert setup.- 312-319: The
prepareEmailRequest
function correctly gathers the email channel configuration from the state. Ensure that all fields, especiallyheaders
, are sanitized and validated to prevent injection attacks or misconfiguration.- 323-347: The
onEmailHandler
function is responsible for saving the email channel configuration. It's important to handle API errors gracefully and provide clear feedback to the user. Additionally, consider adding telemetry or logging to capture the success or failure of email channel creation for debugging and monitoring purposes.- 395-395: The integration of the
onEmailHandler
into thefunctionMapper
ensures that the email channel can be saved using the generic save handler. This approach maintains consistency in how alert channels are handled.- 450-453: The integration of the
prepareEmailRequest
function into theperformChannelTest
switch case allows for testing the email channel configuration. Ensure that the test functionality accurately reflects the email sending process and provides meaningful feedback on failures.- 518-518: The inclusion of
EmailInitialConfig
in theinitialValue
prop ofFormAlertChannels
ensures that the email channel form is pre-populated with default values. Review these defaults to ensure they are sensible and secure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for deployment related changes.
I would like to see this one getting merged as well. |
Summary
Fixes #1248
Fixes #2279
Fixes #623
Part of #2562
Summary by CodeRabbit
Onboarding
andChatSupport
features.AlertChannelMsTeams
for Pro and Enterprise plans.